Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make set trigger outputs in order #47

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Make set trigger outputs in order #47

merged 2 commits into from
Mar 11, 2024

Conversation

wxtim
Copy link

@wxtim wxtim commented Mar 11, 2024 •
edited
Loading

What has changed in this branch.

Cylc set sorts outputs before emitting them.

What problem does this solve

if you run a simple workflow:

[scheduling]
    cycling mode = integer
    [[graph]]
        R1 = """
            foo:a & foo:b => bar
        """

[runtime]
    [[foo]]
        script = sleep 300
        [[[outputs]]]
            a = 'apple (poisoned)'
            b = 'banshee'
    [[bar]]

set it running then

cylc set set/basic//1/foo    # --out=required

gets the following message:

2024-03-11T13:48:42Z INFO - [1/foo/01:submitted] setting implied output: started
2024-03-11T13:48:42Z INFO - [1/foo/01:submitted] => succeeded
2024-03-11T13:48:42Z WARNING - [1/foo/01:succeeded] did not complete required outputs: ['a', 'b']
2024-03-11T13:48:42Z INFO - [1/foo/01:succeeded] completed output a
2024-03-11T13:48:42Z INFO - [1/bar:waiting(runahead)] added to active task pool
2024-03-11T13:48:42Z INFO - [1/bar:waiting(runahead)] => waiting
2024-03-11T13:48:42Z INFO - [1/foo/01:succeeded] completed output b

Confusing IMO.

@wxtim
Make set trigger outputs in order
45b24cc 
Prevent warning messages caused by succeeded being triggered
before custom outputs.
@wxtim wxtim mentioned this pull request Mar 11, 2024
Merged
16 tasks
@oliver-sanders
Copy link

Gets rid of the erroneous warning 👍

@oliver-sanders
Copy link

oliver-sanders commented Mar 11, 2024 •
edited
Loading

If you set multiple outputs on a task, then they will be set one by one. After each output is set, tasks may spawn, have their runahead status changed or be queued. This makes the logs a bit hard to read as log entries relating to the outputs being set are dispersed amongst log entries from other activities.

E.G, this section of log:

2024-03-11T13:57:36Z INFO - Command "set" received. ID=3003469a-d82d-4434-9d28-ff0950dc87d3
    set(flow=['all'], flow_wait=False, outputs=[], prerequisites=[], tasks=['1/foo'])
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] completed output a
2024-03-11T13:57:36Z INFO - [1/bar:waiting(runahead)] added to active task pool
2024-03-11T13:57:36Z INFO - [1/bar:waiting(runahead)] => waiting
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] completed output b
2024-03-11T13:57:36Z INFO - [1/bar:waiting] added to active task pool
2024-03-11T13:57:36Z INFO - [1/bar:waiting] => waiting(queued)
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] setting implied output: submitted
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] setting implied output: started
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] => succeeded(queued)
2024-03-11T13:57:36Z INFO - [1/foo/00:succeeded(queued)] task completed
...
2024-03-11T13:57:36Z INFO - Command "set" actioned. ID=3003469a-d82d-4434-9d28-ff0950dc87d3

Would be better written like so:

2024-03-11T13:57:36Z INFO - Command "set" received. ID=3003469a-d82d-4434-9d28-ff0950dc87d3
    set(flow=['all'], flow_wait=False, outputs=[], prerequisites=[], tasks=['1/foo'])
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] completed output a
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] completed output b
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] setting implied output: submitted
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] setting implied output: started
2024-03-11T13:57:36Z INFO - [1/foo:waiting(queued)] => succeeded(queued)
2024-03-11T13:57:36Z INFO - [1/foo/00:succeeded(queued)] task completed
2024-03-11T13:57:36Z INFO - Command "set" actioned. ID=3003469a-d82d-4434-9d28-ff0950dc87d3
2024-03-11T13:57:36Z INFO - [1/bar:waiting(runahead)] added to active task pool
2024-03-11T13:57:36Z INFO - [1/bar:waiting(runahead)] => waiting
2024-03-11T13:57:36Z INFO - [1/bar:waiting] added to active task pool
2024-03-11T13:57:36Z INFO - [1/bar:waiting] => waiting(queued)

@hjoliver, would it be possible (follow-on work) to do all the output setting first, and delay the downstream consequences until the end of the routine to avoid this?

@hjoliver hjoliver mentioned this pull request Mar 11, 2024
Open
@hjoliver
Copy link
Owner

@hjoliver, would it be possible (follow-on work) to do all the output setting first, and delay the downstream consequences until the end of the routine to avoid this?

Yeah - see cylc#6018

hjoliver
hjoliver approved these changes Mar 11, 2024
View reviewed changes
Copy link
Owner

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wxtim

@hjoliver hjoliver merged commit b858d06 into hjoliver:cylc-set-task Mar 11, 2024
26 of 27 checks passed
@wxtim wxtim deleted the cylc-set-task.output-order branch March 12, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MetRonnie MetRonnie MetRonnie left review comments

@hjoliver hjoliver hjoliver approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wxtim @oliver-sanders @hjoliver @MetRonnie